Conversation
|
@Nahyun-Kang is attempting to deploy a commit to the Eujin Ahn's projects Team on Vercel. A member of the Team first needs to authorize it. |
ParkSohyunee
left a comment
There was a problem hiding this comment.
나현님, 어드민 요청주제 페이지 구현하시느라 고생많으셨습니다! 🥹✨ 리뷰 남긴 부분 확인 부탁드립니다~! 감사합니다. 💕LGTM💕
src/app/admin/topics/page.css.ts
Outdated
| export const body = style({ | ||
| width: '100vw', | ||
| width: '100%', | ||
| minHeight: '100vh', | ||
| padding: '16px 16px 120px', | ||
|
|
| export const table = style({ | ||
| maxWidth: '850px', | ||
| padding: '1rem', | ||
|
|
||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| gap: '1rem', | ||
|
|
||
| backgroundColor: vars.color.white, | ||
| borderRadius: '8px', | ||
| }); | ||
|
|
There was a problem hiding this comment.
해당 스타일 파일에 사용하지 않는 클래스네임이 많은 것 같아서 제거하면 좋을 것 같습니당!
There was a problem hiding this comment.
@ParkSohyunee 네 소현님! 필요없는 css 코드는 삭제해두었습니다!👀
There was a problem hiding this comment.
해당 파일은 삭제해야 하는 파일이 맞을까요?! 👀
| <td className={styles.buttons}> | ||
| <span className={styles.rowText}>{topic?.isAnonymous ? 'O' : 'X'}</span> | ||
| </td> |
There was a problem hiding this comment.
- 나현님, 클론 받아서 테스트해보니 요청 주제를 '익명'으로 만들었을때도 리스폰스에서는 isAnonymous 필드 값이 false로 보여집니다. 이 부분 수정이 필요할 것 같아요~!
- 요청 주제 생성시 노출값이 default로 true인 것이 맞을까요? 검토 후 보여지는 플로우인것 같아서 확인차 여쭤봅니다.
- (의견) 테이블 제목을 '익명' 대신 만든사람 닉네임을 두는 것이 어떨까요? 리스폰스로 해당값이 내려가기도 하고, 관리자가 주제 요청한 사용자에 대해 알아야할 것 같아서요~! 다만, 익명일때만 사용자아이디(익명 요청) 이런식으로 표현해도 될 것 같아요,,
There was a problem hiding this comment.
- 소현님 2번에 대하여, 어드민 페이지가 완전하게 만들어지지 않았을 때 홈 화면에 요청주제가 뜨게 하기 위해서 잠시 회의에서 isAnonymous 옵션을 모두 false로 하고 바로 보여지게 한 걸로 알고 있습니다! 이 부분은 해당 페이지가 완성되면 동호님께 요청드릴 예정입니다! @ParkSohyunee
There was a problem hiding this comment.
- 1번에 대하여 테스트를 해보니, 익명 요청했는데 false로 보여지네요. 이 부분은 요청 주제 생성시와 관련되어있는 것 같아서, 요청 주제 생성 페이지 담당자의 검토도 필요할 것으로 보입니다..!
| <td> | ||
| <select onChange={handleToggleExposeButton} value={topic?.isExposed ? '공개' : '비공개'}> | ||
| <option>공개</option> | ||
| <option>비공개</option> | ||
| </select> | ||
| </td> |
There was a problem hiding this comment.
(의견) 혹시 요청 주제 관리 페이지에서 토글로 노출상태를 바로 변경하는 기능을 두신 이유가 무엇인지 궁금합니당 👀
공지의 경우는 생성/수정 필드와 노출 관련 API가 분리되어 있는 상황이었는데, 요청 주제 관리는 수정 바텀시트에서 함께 수정할 수 있을 것 같다는 생각이 들어서요~! 중복 코드도 제거할 수 있고, 사용성 면에서도 불편함이 없을 것 같아서 제안드려봅니다!
There was a problem hiding this comment.
소현님 이 부분은 와이어프레임을 보고 그대로 만든 건데 바텀시트 안에 들어있어도 좋을 것 같습니다! 다만 이번 PR 말고 다른 PR로 따로 올려서 서영님과 상의를 거친 후 수정해보겠습니다!
| const [isBottomSheetOpen, setIsBottomSheetOpen] = useState(false); | ||
|
|
There was a problem hiding this comment.
useBooleanOutput() 훅을 사용해서 추후 리팩토링하면 좋을 것 같습니당✨👍
| //페이지네이션 코드 | ||
| // const pages = useMemo(() => Array.from({ length: 5 }, (_, idx) => idx + 1), []); | ||
|
|
src/app/admin/topics/page.tsx
Outdated
| <tbody> | ||
| {topics && topics?.topicsList.map((topic, index) => <AdminTopicBox key={index} topic={topic} />)} | ||
| </tbody> | ||
| </table> |
There was a problem hiding this comment.
나현님, key는 index 대신 id로 수정하는 것이 안전해보입니다. 또한, topic의 타입추론이 안되는데 getAdminTopics API 함수의 리스폰스 타입을 지정해두면 좋을 것 같습니당
| const convertCategoryKorNameToCode = (korName: string) => { | ||
| const category = categories?.find((cat) => cat.korName === korName); | ||
| return category ? category.code : null; // 찾지 못하면 null 반환 | ||
| }; | ||
|
|
||
| const editTopicMutation = useMutation({ | ||
| // mutationFn: () => | ||
| // editAdminTopic({ | ||
| // isExposed, | ||
| // title, | ||
| // categoryCode, | ||
| // }), | ||
| mutationFn: () => | ||
| editAdminTopic({ | ||
| topicId, | ||
| isExposed, | ||
| title, | ||
| categoryCode: convertCategoryKorNameToCode(selectedCategory as string) || '', | ||
| }), |
There was a problem hiding this comment.
추후 리팩토링 하실 때 드롭다운을 ul, li태그가 아닌 select 태그를 사용하는 것으로 수정하면 카테고리 선택하고, 뮤테이션에 넣어주는 부분에 대해 코드 가독성면에서 좋을 것 같아요, 아래 코드 참고 부탁드립니당
// 제거
// const convertCategoryKorNameToCode = (korName: string) => {
// const category = categories?.find((cat) => cat.korName === korName);
// return category ? category.code : null; // 찾지 못하면 null 반환
// };
const editTopicMutation = useMutation({
mutationFn: () =>
editAdminTopic({
...
categoryCode: selectedCategory, // 수정
}),
...
},
});
return (
<select ... onChange={(e: ChangeEvent<HTMLSelectElement>) => setSelectedCategory(e.target.value)}>
{categories?.map((category) => (
<option ... value={category.code}>{category.korName}</option>))}
</select>
)
There was a problem hiding this comment.
리팩토링 시 꼭 코드 반영하겠습니다. 조언 감사합니다 소현님!

개요
작업 사항
@ParkSohyunee
참고 사항 (optional)
관련 이슈 (optional)
스크린샷
리뷰어에게